Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor InstallServer function to integrate DepotDownloader better #664

Merged
merged 4 commits into from
Jan 21, 2025

Conversation

sonroyaalmerol
Copy link
Contributor

Context

This is mostly to help #660 by updating DepotDownloader to the latest version and reorganize the installation script. I couldn't recreate the issue on my own ARM64 machine so I'm not sure if this would solve it. I'm hoping it has something to do with this change in the latest version:

Do not abort downloading if one depot fails to get depot key

At this point, DepotDownloader is our only option for Apple CPUs (and probably other ARM64 processors) to download the server files. Refactoring the script to integrate it with steamcmd even if it's not explicitly enabled will probably be a net positive for the user experience.

Choices

  • Force ARM64 hosts without 4k page size to use DepotDownloader because steamcmd will never work on them anyways.
  • Always fallback to DepotDownloader for all steamcmd commands that fail (non-exit code 0). I don't think there's any disadvantage to doing this at all.
  • Check PalServer.sh existence before creation of PalServer-arm64.sh to prevent "non user-friendly" errors returned by the cp, sed, and chmod commands. This is just purely for better experience and log flow.
  • I also added -NumberOfWorkerThreadsServer=$(nproc --all) while I'm at it as suggested in Orange Pi 5 Max Performance improvement #640 since it's a pretty small change for an entirely new PR.

Test instructions

  1. Ran the container in both architectures without the /palworld folder
  2. Ran the container in both architectures with the /palworld folder

Checklist before requesting a review

  • I have performed a self-review/test of my code
  • I've added documentation about this change to the docs.
  • I've not introduced breaking changes.
  • My changes do not violate linting rules.

@sonroyaalmerol sonroyaalmerol marked this pull request as draft January 19, 2025 23:03
@sonroyaalmerol sonroyaalmerol marked this pull request as ready for review January 19, 2025 23:24
@sonroyaalmerol
Copy link
Contributor Author

This should be ready to go. The amd64 unit test seems to need more timeout as it's taking longer for it to download the server for some reason.

@thijsvanloef thijsvanloef merged commit 1d9cd9a into thijsvanloef:main Jan 21, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants